Skip to content

Update zend_gdb.c #11599

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 1 commit into from
Closed

Update zend_gdb.c #11599

wants to merge 1 commit into from

Conversation

onthefly
Copy link

@onthefly onthefly commented Jul 5, 2023

This mistyping prevents building php from source under FreeBSD

This mistyping prevents building php from source under FreeBSD
@devnexen
Copy link
Member

devnexen commented Jul 5, 2023

are you sure about your change tough ? the failed FreeBSD CI is related.

@devnexen
Copy link
Member

devnexen commented Jul 5, 2023

Note kinfo_getproc expects a process id so the kinfo_proc's ki_tracer field represents the tracing process id. I may suggest to open an issue instead describing in detail your issue (including eventually cflags and all of that) we may be able to pinpoint the root cause.

@onthefly
Copy link
Author

onthefly commented Jul 8, 2023

are you sure about your change tough ? the failed FreeBSD CI is related.

i tried to build php-7.4.33, php-8.0.29 and php-8.1.20 and it always fails: screenshot of error

Pls note: it's FreeBSD 9.2

@devnexen
Copy link
Member

devnexen commented Jul 8, 2023

Couple of things.

  • the compiler proposes a fix for a supposed typo. However, does not necessarily means it s the right choice. The ki_tracep field is completely unrelated as you can see here.
  • note that freebsd 9.2 is EOL since the end of 2014 and indeed at the time the ki_tracer field did not exist yet (only from FreeBSD 11). Any particular reason for not using a more recent release ?

Eventually the fix would be to disable it altogether for FreeBSD < 11 (via __FreeBSD_version__).

devnexen added a commit to devnexen/php-src that referenced this pull request Jul 9, 2023
@iluuu1994
Copy link
Member

Let's close this in favor of #11646, thanks!

@iluuu1994 iluuu1994 closed this Jul 13, 2023
devnexen added a commit that referenced this pull request Jul 13, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants